Code Review Finding 1/7 — Bug: imageId regex permits _ but split('_') will misparse the key

File: api/utils/index.ts, line 22 (not in diff — bug pre-exists, but the new test at integrationTests/image-optimizer.test.ts:121 uses put-test-image which contains hyphens, not underscores, so this gap is not caught by any test)

Problem:

if (!imageId || !/^[a-z0-9_-]+$/i.test(imageId)) return null;  // ← allows _

parseCacheKey splits on '_' and requires exactly 4 parts. Any imageId containing an underscore (e.g. my_photo) produces 5+ parts → the function returns null → callers throw 400. This means:

• GET /ImageVariant/my_photo_300_2_webp → 400 Malformed cache key (instead of serving the variant)
• PUT /Images?id=my_photo → variant purge silently skips all variants (parseCacheKey used in allowRead)

The regex says underscores are valid in an imageId, but the parser will always reject them.

Fix (Option 1 — simplest, disallow _ in imageId):

if (!imageId || !/^[a-z0-9-]+$/i.test(imageId)) return null;

Fix (Option 2 — allow _, split from the right):

const lastUnder3 = rawId.lastIndexOf('_', rawId.lastIndexOf('_', rawId.lastIndexOf('_') - 1) - 1);
const imageId = rawId.slice(0, lastUnder3);
const [widthRaw, dprRaw, formatRaw] = rawId.slice(lastUnder3 + 1).split('_');

Code Review Finding 4/7 — Plausible: sharp(failOnError: false) silently stores a corrupt zero-byte variant

File: api/resources.ts, line 97 (not in diff)

let sharpInstance = sharp(originalBuf, { failOnError: false });

failOnError: false tells sharp to treat warnings (e.g. a partial/truncated JPEG) as non-fatal. An image that passes sharp(bytes).metadata() at upload time can still fail during a resize/encode operation — sharp will silently emit a degraded or zero-byte buffer instead of throwing. That buffer gets stored via VariantsTable.put(record) and returned as a 200 response, so the corrupt variant is cached and served on every subsequent HIT.

Suggested fix: Remove failOnError: false so sharp throws on corrupt inputs, or explicitly validate variantBuf.length > 0 before persisting:

if (variantBuf.length === 0) {
    const err: any = new Error('Variant generation produced empty buffer');
    err.statusCode = 500;
    throw err;
}

Code Review Finding 5/7 — Cleanup: Duplicate byte-extraction logic in post() and put()

File: api/resources.ts, lines 208–213 and 257–262

Both Images.post() and Images.put() contain identical 3-branch byte-extraction blocks:

if (Buffer.isBuffer(data?.data)) {
    bytes = data.data;
} else if (typeof data?.data?.arrayBuffer === function) {
    bytes = new Uint8Array(await data.data.arrayBuffer());
} else if (data?.data) {
    bytes = new Uint8Array(data.data);
}

Extract this into a shared helper to reduce duplication and make it easier to fix uniformly if the data shape changes:

async function extractBytes(data: any): Promise<Buffer | Uint8Array | undefined> {
    if (Buffer.isBuffer(data?.data)) return data.data;
    if (typeof data?.data?.arrayBuffer === 'function') return new Uint8Array(await data.data.arrayBuffer());
    if (data?.data) return new Uint8Array(data.data);
    return undefined;
}

Code Review Finding 6/7 — Efficiency: Unnecessary setImmediate yields in ImageVariant.get()

File: api/resources.ts, lines 65 and 122

// line 65 — after cache miss
await new Promise((resolve) => setImmediate(resolve));

// line 122 — before sharpInstance.toBuffer()
await new Promise((resolve) => setImmediate(resolve));

Both of these yield the event loop on every cache-miss variant generation. The second is particularly unnecessary: sharpInstance.toBuffer() is already fully async (it runs in libuv's thread pool), so the event loop is never blocked by it. The first yield adds a full event-loop tick of latency before every image load.

Neither yield is needed to keep the server responsive. Consider removing them, or at minimum remove the one at line 122:

// Remove: await new Promise((resolve) => setImmediate(resolve));
variantBuf = await sharpInstance.toBuffer();

Review follow-up (autonomous agent): Fixed all three blocking findings: removed _ from imageId regex to align with split-based parsing, added npm test step to CI to preserve unit test coverage, and pinned harper to 5.0.28 and @harperfast/integration-testing to 0.4.0 for reproducible builds.